ci: replace lerna + yarn + CircleCI with pnpm and npm trusted publishing - #87

Single squashed commit of ci/pnpm-trusted-publishing (#87), on the assumption
that #87 lands on main before this PR. Purpose is measurement: the pnpm
migration shifts CodSpeed's baseline on its own, and the HTJ2K work in this
branch shifts it again, so carrying both here lets one report show the combined
effect instead of attributing the sum to whichever merges second.

Expect this commit to become a no-op the moment #87 merges -- it should then
either drop out of the diff or merge cleanly against itself. It is NOT a second
copy of that work to review; review it in #87.

Merged with no conflicts. Two things worth noting about the overlap:

  - The submodule gitlink stayed at this branch's 0748112b rather than taking
    #87's e01c7b7, because #87 only reverted its own accidental bump back to
    the value main already had. Updated separately in the next commit.

  - dicom-codec/src/codecs/codecFactory.js is touched by both branches and did
    not conflict: #87 changes initialize() (routing emscripten's print through
    the logger) while this branch's carried work from #68 changes decode()
    (decoder reuse). They are independent edits to the same file.

Includes the pnpm.overrides pinning esbuild/rollup/webpack/terser to the
versions yarn.lock resolved, so build output does not drift across the
migration -- relevant here because this PR is measured against those baselines.

Carried from #68. codecFactory.decode gains an opt-in reuseDecoder option: the
decoder is held on codecConfig (the per-codec singleton the wrapper modules
already share) and not deleted after each call. htj2k.js opts in; every other
codec keeps the construct-and-delete behaviour.

This is very likely the bulk of #68's measured 3.5x speedup on the dicom-codec
dispatch bench for HTJ2K -- CodSpeed reported 141.4ms -> 40.6ms there, and 40.6ms
is about what openjphjs' own direct decode benches cost, i.e. reuse closes the
gap between dispatching through this factory and calling the codec directly.
Constructing a wasm decoder per frame allocates heap and registers embind
bindings each time; for openjph it also ran the constructor banner through the
console on every frame.

Opt-in rather than default on purpose: a decoder that carries state between
decodes, or whose retained buffers grow without bound, must not enable it.
openjphjs' reuse test covers the consequence that matters for HTJ2K -- 500
successive decodes on one instance without progressive slowdown.

Independent of #87's change to the same file: that one routes emscripten's print
through the logger in initialize(); this one changes decode(). They merged with
no conflict.

This branch was cut from a July main and had not seen main's later commits, so
squashing #87 in (2a9e8b7) brought content for five files that main gained in
the meantime, and GitHub reported the PR as CONFLICTING. That blocked CI
entirely -- pull_request workflows do not run when the merge commit cannot be
created -- so no checks had run on the integration.

Resolved all five in favour of this branch, verified rather than assumed: each
was already byte-identical to ci/pnpm-trusted-publishing's version, which is
main's content plus #87's edits, so taking ours preserves both sides.

  .github/CODEOWNERS
  .github/workflows/bench.yml
  .github/workflows/pr-checks.yml
  docs/ci/self-hosted-runner.md
  tools/ci/with-nashua-lock.sh

Confirmed intact afterwards: the submodule still points at 4a68609 (fork PR #6),
set_message_level in jslib.cpp, the OJPH_WARN diagnostics, reuseDecoder in
codecFactory, truncated.test.js, DISABLE_EXCEPTION_CATCHING=0, and yarn.lock /
lerna.json still deleted with the pnpm files in place.

…ersedes #68) (#76)

codspeed regression is due to parallelization not being consistent in the simulation, not due to code speed changes

* chore(openjphjs): bump extern/openjph submodule to upstream 0.30.1

Points the openjph submodule at cornerstonejs/OpenJPH#5, which rebases the
fork onto upstream OpenJPH 0.30.1 (was ~22 months behind) and re-applies our
custom patches. Net cornerstonejs delta from stock 0.30.1 is 3 lines in
ojph_codestream_local.cpp: resilient=true (tolerate truncated streams) +
suppressed 'File terminated early' log. Dropped the cosmetic SIZ-marker
message renames (conflicted with upstream's swap_byte rename) and the
temporary debug-build toggle.

CI is the first real build/validation of this bump (not built locally).
After OpenJPH#5 merges, re-point this submodule at the merge commit.

* fix(openjphjs): add OpenJPH 0.30.1 header dirs to the wasm target include path

0.30.1 relocated its public headers under src/core/openjph (+ src/core/shared);
our glue's bare <ojph_arch.h> include no longer resolved and the openjphjs wasm
build failed with 'ojph_arch.h file not found'. Add both 0.30.1 header roots to
the openjphjs target's include path.

* fix(openjphjs): link the OpenJPH 0.30.1 'openjph' target (was renamed from openjphsimd)

0.30.1 builds a single architecture-agnostic 'openjph' library; the old
'openjphsimd' target no longer exists, so wasm-ld failed with
'unable to find library -lopenjphsimd'. Link 'openjph', matching upstream's
own wasm wrapper (subprojects/js).

* fix(openjphjs): keep WASM SIMD enabled for OpenJPH 0.30.1

0.30.1 deprecated OJPH_DISABLE_INTEL_SIMD and bridges it onto the new
OJPH_DISABLE_SIMD; our old 'OJPH_DISABLE_INTEL_SIMD=ON' therefore disabled ALL
SIMD (OJPH_ENABLE_WASM_SIMD=OFF), shipping a scalar wasm ~2x slower on decode/
encode. Stop setting the deprecated option and force OJPH_DISABLE_SIMD=OFF so
0.30.1's Emscripten path builds the WASM SIMD kernels (-msimd128).

* fix(openjphjs): build the wasm in Release, not Debug

build.sh forced CMAKE_BUILD_TYPE=Debug, so the shipped openjph wasm was built
-O0 with unoptimized SIMD intrinsics — the reason decode/encode benched far
slower (SIMD-on was even slower than scalar under -O0) and the binary was
oversized. Release (-O3) is the correct artifact and is what makes the 0.30.1
SIMD kernels fast and the wasm small.

* ci: squash in the pnpm migration from #87

Single squashed commit of ci/pnpm-trusted-publishing (#87), on the assumption
that #87 lands on main before this PR. Purpose is measurement: the pnpm
migration shifts CodSpeed's baseline on its own, and the HTJ2K work in this
branch shifts it again, so carrying both here lets one report show the combined
effect instead of attributing the sum to whichever merges second.

Expect this commit to become a no-op the moment #87 merges -- it should then
either drop out of the diff or merge cleanly against itself. It is NOT a second
copy of that work to review; review it in #87.

Merged with no conflicts. Two things worth noting about the overlap:

  - The submodule gitlink stayed at this branch's 0748112b rather than taking
    #87's e01c7b7, because #87 only reverted its own accidental bump back to
    the value main already had. Updated separately in the next commit.

  - dicom-codec/src/codecs/codecFactory.js is touched by both branches and did
    not conflict: #87 changes initialize() (routing emscripten's print through
    the logger) while this branch's carried work from #68 changes decode()
    (decoder reuse). They are independent edits to the same file.

Includes the pnpm.overrides pinning esbuild/rollup/webpack/terser to the
versions yarn.lock resolved, so build output does not drift across the
migration -- relevant here because this PR is measured against those baselines.

* build(openjphjs): track upstream OpenJPH master for the streaming fix

Moves extern/openjph from 0.30.1 + carried patches to cornerstonejs/OpenJPH#6,
which merges upstream master (6f3caf3) with ZERO fork delta.

Why master rather than a release: the streaming/truncated-decode fix landed
upstream as 638ccb4 "Cs3d/truncated decode graceful 0.30.1 (aous72#331)" on
2026-08-08, and the newest upstream release 0.31.0 was published 2026-07-27 --
twelve days earlier. `git tag --contains 638ccb4` is empty, so no tagged release
carries it yet. master is four commits past the fix (three dependabot codeql
bumps and a warning fix). Re-pin to a tag once upstream cuts one with #331.

Both patches the fork used to carry are gone, replaced by public API:

  resilient = true
      codestream::enable_resilience() -- already called by HTJ2KDecoder on main
      (line 270), so this patch was redundant before this PR.

  commented-out OJPH_INFO "File terminated early"
      ojph::set_message_level(OJPH_MSG_WARN), set here in jslib.cpp.

The message level is worth its own note. OpenJPH INFO goes to stdout, which
emscripten forwards to console.log, and there were two sources of per-decode
noise: HTJ2KDecoder's constructor banner ("v06 HTJ2K Decoder") on every
construction, and "File terminated early" on every resilient decode of a
truncated stream -- which with streaming support is the normal case. Raising the
threshold to WARN drops both and keeps warnings and errors, so it replaces a
source patch with a supported call and makes future upstream bumps
fast-forwards.

Verified: OJPH_DISABLE_SIMD still exists upstream (this branch's FORCE OFF is
still correct), and every header HTJ2KDecoder.hpp includes is present under
src/core/openjph on master.

* fix(openjphjs): decode truncated HTJ2K streams without a known full length

Carried from #68, which this PR supersedes. Previously the decoder could handle
a partial HTJ2K stream only if the caller already knew the full length; now a
truncated buffer decodes as far as its data allows.

readHeader, decode and decodeSubResolution wrap their codestream work in
try/catch and report instead of propagating, so resilient mode's throw at the
end of the available data yields a partial image rather than a failed decode.
frameInfo_ keeps whatever the header established, so dimensions survive.

Two deliberate changes from #68's version:

  - The diagnostics are OJPH_WARN, not OJPH_INFO. jslib.cpp raises OpenJPH's
    threshold to WARN to kill the per-construction banner, so INFO here would be
    dropped exactly when a decode failed.

  - DISABLE_EXCEPTION_CATCHING flips 1 -> 0 (double negative: catching ENABLED).
    This is required, not stylistic: with catching disabled emscripten compiles
    the handlers out and the throw terminates the module instead of being
    caught. It costs wasm size, so dist-size may need re-baselining.

test/truncated.test.js covers truncated and lossy decodes, and decoder reuse
across 500 decodes. NOTE: its performance assertions are wall-clock
(reused-faster-than-fresh, and a min/max ratio across milestones), so they are
inherently softer than the pixel-exactness tests and may prove flaky on shared
CI runners. Worth watching, and worth converting to a looser bound or dropping
if they turn noisy.

The core-side work is upstream as of aous72#331, so this is only the emscripten
wrapper plus tests -- the corresponding fork patches are gone.

* perf(dicom-codec): reuse the HTJ2K decoder instead of one per frame

Carried from #68. codecFactory.decode gains an opt-in reuseDecoder option: the
decoder is held on codecConfig (the per-codec singleton the wrapper modules
already share) and not deleted after each call. htj2k.js opts in; every other
codec keeps the construct-and-delete behaviour.

This is very likely the bulk of #68's measured 3.5x speedup on the dicom-codec
dispatch bench for HTJ2K -- CodSpeed reported 141.4ms -> 40.6ms there, and 40.6ms
is about what openjphjs' own direct decode benches cost, i.e. reuse closes the
gap between dispatching through this factory and calling the codec directly.
Constructing a wasm decoder per frame allocates heap and registers embind
bindings each time; for openjph it also ran the constructor banner through the
console on every frame.

Opt-in rather than default on purpose: a decoder that carries state between
decodes, or whose retained buffers grow without bound, must not enable it.
openjphjs' reuse test covers the consequence that matters for HTJ2K -- 500
successive decodes on one instance without progressive slowdown.

Independent of #87's change to the same file: that one routes emscripten's print
through the logger in initialize(); this one changes decode(). They merged with
no conflict.

* test(openjphjs): make the decoder-reuse perf assertion measure something real

Built openjphjs locally via tools/docker/build.sh and ran the suite, which is
how this surfaced: "reused decoder is faster than instantiate+decode+destroy per
frame" FAILED locally (3.34 ms vs 2.72 ms) while passing CI by 5% (2.38 vs 2.50).

The original had a structural flaw, not bad luck. It took ONE sample per path
with no warmup, and measured the reused path FIRST -- so V8's JIT warmup was
charged to exactly the side the assertion expects to win. Construction costs well
under a millisecond against a ~2.5 ms decode, so a single cold sample measures
warmup rather than the difference under test.

Fixed the measurement: warm both paths, then compare medians of 25 iterations.
That removed the order bias but showed the assertion itself is not sound at this
granularity -- warmed, construct+decode+destroy costs about the same as decode
alone (~1.6 ms each), so the medians sit inside each other's noise. Eight
observed runs produced two failures on unchanged code.

So the assertion is now a bound in the useful direction: reuse must not be
materially SLOWER (the real risk, e.g. retained state degrading each decode)
rather than provably faster. The medians are still logged.

This does not weaken the perf claim, it relocates it to the tool that can
actually measure it. CodSpeed on this branch reports the dispatch bench
141.5 ms -> 24.1 ms and instantiate+destroy HTJ2KDecoder x50 2315 us -> 458 us,
because Simulation counts instructions where wall-clock at ~3% of a decode
cannot resolve it. The 500-decode stability test is untouched and still guards
the thing that matters for reuse: no progressive slowdown from retained buffers.

Verified: 5 consecutive local runs stable, full openjphjs suite 30 passed.

* fix(htj2k): own the decoded buffer and surface swallowed decode failures

Addresses the review findings on #76, all of which stem from the same two
changes in this PR: reusing one HTJ2K decoder across a series, and swallowing
OpenJPH's exceptions so a partial codestream degrades to a partial image.

Buffer ownership (the critical one). getDecodedBuffer()/getEncodedBuffer()
return an emscripten typed_memory_view -- a live window onto the wasm heap
owned by the codec instance. Returning it as `imageFrame` was wrong three ways:
delete() frees the memory it points at, the next decode on a reused instance
overwrites it, and heap growth detaches it outright. Frames 1..n of a series
all showed frame n. copyFromWasm() now copies on both the decode and encode
paths -- unconditionally, because the non-reuse path was already handing back
memory delete() had just freed. Measured side effect: the raw view's .buffer is
the whole 50 MB heap, so callers passing imageFrame.buffer to a worker were
transferring the heap rather than the frame.

Failure reporting. decode()/readHeader()/decodeSubResolution() returned
normally after swallowing an exception, so codecFactory reported success and
the OJPH_WARN went to a logger that is silent unless setVerbose. HTJ2KDecoder
now exposes getIsHeaderValid()/getLastErrorMessage(), reset per call, and
codecFactory throws on an invalid header while flagging processInfo.partial
otherwise. Verified: before this, garbage input on a reused decoder resolved
successfully with the previous slice's pixels under the new frame's metadata.

Stale pixels. decode_ used resize(), which only value-initialises NEW elements,
so anything the decoder did not write kept the previous frame's pixels. Now
assign(size, 0), which zero-fills without giving up the capacity reuse depends
on. readHeader_ likewise resets every header-derived field before parsing, and
the previously uninitialised members (numDecompositions_, numLayers_, ...) get
initialisers -- a failed parse on a fresh decoder was doing arithmetic on heap
garbage.

Two comments in this PR claimed things that turned out to be false, corrected
in place. Truncation does NOT throw into decode()'s catch: swept CT1.j2c at
every length from 60 bytes up plus 875 single-byte corruptions and not one
input aborts mid-decode -- resilient mode absorbs a short codestream as zero
coefficients and reports success. The reachable stale-buffer window is
restrict_input_resolution() throwing for a decomposition level the codestream
does not carry, which is what the new regression test uses (it fails against
resize() with 128 stale bytes; a truncation-based test passes either way and
tests nothing).

Also:
- unset(OJPH_DISABLE_INTEL_SIMD CACHE): deleting the option() line does not
  remove it from an existing CMakeCache.txt, and upstream's `DEFINED` bridge
  shadows the forced OJPH_DISABLE_SIMD=OFF, so incremental local builds kept
  shipping the scalar wasm this PR's own comment warns about.
- releaseDecoder()/htj2k.release()/dicomCodec.release(): a reused decoder held
  its largest frame's buffers for the module's lifetime with no way to free.
- dist-size baseline for openjphjs: stale at 2241 KiB against a 293 KiB
  artifact, because THIS PR switched the build Debug -> Release. The gate only
  fails on growth, so it would have tolerated a 7.7x regression.

Rebuilt with tools/docker/build.sh (emsdk 3.1.74, SIMD confirmed intact);
openjphjs 34 passed, dicom-codec 41 passed, other codec suites unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

…TOKEN

The release has been broken since #87. `git push HEAD:main` is declined --
`protected branch hook declined` -- because main requires a pull request and the
built-in GITHUB_TOKEN has no exemption from it.

tools/release/setup-branch-ruleset.sh was written to fix that by migrating to a
ruleset listing the "GitHub Actions" app as a bypass actor. That cannot work.
App 15368 is owned by `github`, not by this org, and a repository ruleset only
accepts bypass actors belonging to the repo or its owning organization, so the
API refuses it:

  422 Actor GitHub Actions integration must be part of the ruleset source or
      owner organization

There is no repository setting that grants GITHUB_TOKEN a push to a
PR-protected branch. An org-owned GitHub App is the supported route, and unlike
a PAT it belongs to the org rather than to a person, so releases do not break
when someone's token expires or they leave -- which was the whole point of
moving off the maintainer SSH key CircleCI used.

release.yml now mints a token per run from that App via
actions/create-github-app-token, gated on `vars.RELEASE_APP_ID` so it no-ops on
forks and before the one-time setup. When the App is not configured the job
still runs and still fails at the push, but logs a warning naming the setup doc
instead of leaving the next person to re-derive all of the above from
"protected branch hook declined".

setup-branch-ruleset.sh takes the App slug, resolves its id, and refuses to
continue if the App is not owned by the org -- failing locally with an
explanation rather than letting GitHub return the 422 this script exists to
avoid. It also warns when the App is not installed on the org, since that yields
a ruleset that looks correct and still cannot push. Its header carries the full
org-owner UI walkthrough. Uses gh's built-in --jq throughout: this runs on a
maintainer's laptop, where standalone jq is not a given.

Does not change what humans need to merge: the ruleset reproduces main's current
rules exactly (1 approving review, code-owner review, dismiss stale on push,
require last-push approval, no force push, no deletion).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…he CodSpeed bench (#89)

* ci: measure the CodSpeed simulation benches one package at a time

The simulation gate ran all eight packages' bench processes concurrently on the
shared nashua box, on the premise -- stated in codspeed-walltime's own comment
-- that instruction counting is immune to contention. #76 showed it is not.

There, `decode CT-512x512-near-lossless.JLS (.81 near-lossless) — warm` was
reported as a 19.8ms -> 37.9ms regression (-47.76%) on a commit whose entire
diff was one vitest file. charls' source and its built wasm were byte-identical
to main's, its real wall-clock bench duration was unchanged (26.7s vs 27.5s),
and the identical -47.76% reappeared on the following commit, so it was
reproducible rather than flake. Per-package completion times from that run put
charls at 27s in, sharing the box with six or seven siblings, while dicom-codec
then ran alone for ~5m54s: the packages are measured under wildly different
neighbours, and #76 changed what those neighbours do (its openjph benches got
3-7.4x faster). Whatever the mechanism inside Cachegrind, a gate that measures
eight packages simultaneously cannot attribute a per-package delta -- and it
spent #76 blaming a package the PR never touched.

--workspace-concurrency=1 rather than just dropping --parallel: pnpm's default
workspace concurrency is 4, so removing the flag alone would still have run four
bench processes against each other. Verified the distinction directly -- with
the flag, three filtered packages run strictly back to back; with --parallel all
three start within 25ms and overlap. This also makes the two CodSpeed jobs use
one idiom, since codspeed-walltime already serialises this way.

Cost is about 3 minutes: dicom-codec alone is ~6m of the current 6m22s bench
step, and the job timeout is 100 minutes.

Landing this resets the comparison basis for every bench previously measured
under contention, so the first main run after merge is the new baseline --
expect one round of large apparent deltas there and nothing to act on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* ci: push release refs atomically so rejected branch updates leave no tags

`git push --follow-tags HEAD:main` updates each ref independently. The
2026-08-24 release run (32733067241) showed the failure mode: main's protected
branch hook declined `HEAD -> main`, but all eight version tags pushed
successfully anyway, leaving them on a `chore(release): publish` commit that is
not an ancestor of main.

That turned a clean, retryable failure into a wedged repo. Every later release
run died at `git tag -a` with "tag '@cornerstonejs/dicom-codec@1.0.12' already
exists" -- earlier than the real problem and with a misleading message -- and
recovery required a human deleting eight remote tags. Nothing had been
published; npm latest still matched main's manifests throughout.

--atomic makes all refs land or none. A rejected branch update can no longer
publish tags for a release that did not happen.

This does not fix the underlying rejection: the token needs a genuine bypass on
main's ruleset, which is a repo-admin change and is being handled separately.
It makes the next failure recoverable by re-running rather than by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* ci: push the release commit with an org-owned GitHub App, not GITHUB_TOKEN

The release has been broken since #87. `git push HEAD:main` is declined --
`protected branch hook declined` -- because main requires a pull request and the
built-in GITHUB_TOKEN has no exemption from it.

tools/release/setup-branch-ruleset.sh was written to fix that by migrating to a
ruleset listing the "GitHub Actions" app as a bypass actor. That cannot work.
App 15368 is owned by `github`, not by this org, and a repository ruleset only
accepts bypass actors belonging to the repo or its owning organization, so the
API refuses it:

  422 Actor GitHub Actions integration must be part of the ruleset source or
      owner organization

There is no repository setting that grants GITHUB_TOKEN a push to a
PR-protected branch. An org-owned GitHub App is the supported route, and unlike
a PAT it belongs to the org rather than to a person, so releases do not break
when someone's token expires or they leave -- which was the whole point of
moving off the maintainer SSH key CircleCI used.

release.yml now mints a token per run from that App via
actions/create-github-app-token, gated on `vars.RELEASE_APP_ID` so it no-ops on
forks and before the one-time setup. When the App is not configured the job
still runs and still fails at the push, but logs a warning naming the setup doc
instead of leaving the next person to re-derive all of the above from
"protected branch hook declined".

setup-branch-ruleset.sh takes the App slug, resolves its id, and refuses to
continue if the App is not owned by the org -- failing locally with an
explanation rather than letting GitHub return the 422 this script exists to
avoid. It also warns when the App is not installed on the org, since that yields
a ruleset that looks correct and still cannot push. Its header carries the full
org-owner UI walkthrough. Uses gh's built-in --jq throughout: this runs on a
maintainer's laptop, where standalone jq is not a given.

Does not change what humans need to merge: the ruleset reproduces main's current
rules exactly (1 approving review, code-owner review, dismiss stale on push,
require last-push approval, no force push, no deletion).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* chore: drop stray orphan-tags.txt

Scratch output from the one-off tag cleanup (the list of eight version tags
orphaned by run 32733067241), swept in by `git add -A`. Not repo content.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* ci: allow a repo-scoped deploy key as the release push credential

The App route added in the previous commit needs an organization owner to create
and install the App, which is not available here -- Alireza granted repo admin
on codecs, and /organizations/cornerstonejs/settings/apps/new 404s for a member.

A deploy key is the repo-admin-sized answer. `DeployKey` is a valid ruleset
bypass actor per GitHub's rules API, and a deploy key belongs to the repository
by definition, so it satisfies "part of the ruleset source" with none of the
ownership problem that makes app 15368 a 422. Like the App and unlike a PAT it
is not tied to any individual, so releases survive people leaving.

release.yml now resolves a credential in preference order -- App token, then
deploy key over SSH, then GITHUB_TOKEN -- logs which one it picked, and warns
when it lands on the last. The key is written to a mktemp file rather than a
command line and removed on a trap.

The deploy-key push is an ordinary push, unlike the App and GITHUB_TOKEN cases,
so it WOULD retrigger this workflow on main. The `[skip ci]` already in the
release commit message is what stops that being a loop; noted at the push and in
the release doc so it does not get removed as dead weight.

setup-branch-ruleset.sh takes BYPASS=deploy-key (default) or BYPASS=app and
keeps every existing guard for the app path.

One thing worth reviewing rather than just applying: the DeployKey actor takes
`actor_id: null`, so it is a category, not a specific key. EVERY write-enabled
deploy key on the repo -- present and future -- gains a bypass on main. The
script therefore lists them and asks the operator to look, instead of counting
them. This repo currently has a read-write key `Codecs CircleCI` (id 108740348,
2024-09-18); CircleCI runs nothing here any more, so it should be deleted rather
than silently promoted into a credential that bypasses branch protection. That
asymmetry is the deploy key's real cost against the App, and it is now in the
comparison table in tools/release/README.md rather than left for someone to
discover.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* ci: scope the release App token, and gate the deploy-key bypass on an audit

Two review findings on #89.

Accepted: the DeployKey bypass actor is category-wide, and the script only
warned about it. `actor_id: null` means every write-enabled deploy key on the
repo can push to main without review, so creating this ruleset can silently
promote an unrelated credential into a branch-protection bypass. This repo had
exactly that case -- a read-write `Codecs CircleCI` key, years after CircleCI
stopped running here. A warning above a y/N prompt is too easy to scroll past
for an escalation that quiet, so the operator now has to type `audited` after
reading the list of keys (or set DEPLOY_KEYS_AUDITED=1 for non-interactive use).

Also accepted, from zizmor: an App token inherits the installation's blanket
permissions. Added `permission-contents: write` so the release token is narrowed
to what the push needs, and a later widening of the App does not silently widen
the release token.

Rejected: adding continue-on-error to the app-token step so a failed mint falls
through to the deploy key. That trades a loud failure for a silent credential
downgrade -- from a token scoped to Contents: write and expiring in an hour, to
one with write access to the whole repo and no expiry -- in a pipeline that
publishes to npm, detectable only by reading the log of a release that appeared
to succeed. If someone configured the App, the App is what should be used or the
run should stop. The action validates the private key up front and retries
transient 5xx itself, so what reaches that failure is a real misconfiguration,
and --atomic means the failed run leaves nothing to clean up. Recorded as a
comment at the step so it does not get "fixed" later.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: correct which pushes GitHub suppresses workflow runs for

Two review corrections from @jbocce.

The [skip ci] note in release.yml and tools/release/README.md said App
tokens, like GITHUB_TOKEN, do not retrigger workflows, and framed
[skip ci] as mattering "for the deploy key". That is backwards: only
GITHUB_TOKEN pushes are suppressed. An App-token push is an ordinary
push and would retrigger the release workflow on main just as a
deploy-key push would. No bug today -- [skip ci] covers both routes --
but read as written, someone on the App route could conclude it was
redundant and remove it, which loops releases. Reworded so it reads as
load-bearing on both routes.

The deploy-key warning in the README also named `Codecs CircleCI`
(id 108740348) as a key to delete. It has since been deleted; the
release key is now the only write-enabled key on the repo. Left in,
a reader who checks and finds it missing may take the whole warning
for stale and skip the audit, so the specific name and id are gone and
the general rule stays. setup-branch-ruleset.sh keeps its mention --
it is past tense there, explaining why the typed acknowledgement
exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>